EVM: error handling#3714
Conversation
| type EvmRuntimeErrorType = { | ||
| code: EvmErrorCode.RUNTIME_ERROR | ||
| reason: RuntimeErrorMessage | EOFError | ||
| } & ( | ||
| | { reason: RuntimeErrorMessage.REVERT; revertBytes: Uint8Array } | ||
| | { reason: Exclude<RuntimeErrorMessage, RuntimeErrorMessage.REVERT> | EOFError } | ||
| ) |
There was a problem hiding this comment.
Is this scalable? I'm wondering how this would look if we had multiple error codes with corresponding different formats. Right now we're just having one case for "RuntimeErrorMessage.REVERT" and another excluding "RuntimeErrorMessage.REVERT", but I wonder if that might become too burdensome if we need to exclude more
There was a problem hiding this comment.
This was some experimenting with the typescript typing I did. It ensures that if reason is RuntimeErrorMessage.REVERT it has the mandatory revertBytes. If is NOT a REVERT then the reason could be any of the RuntimeErrorMessage or an EOFError.
Note: EOFError has not been migrated to the EvmError, likely when I do that this typing (the second line) does not have to be there (will check)
There was a problem hiding this comment.
This typing (I checked) is just to type the evm errors such that the REVERT error has a revertBytes field. It is straightforward to add new error codes or new error reasons, or the inject specific error which adds context (like the mandatory revertBytes field in the REVERT error)
| constructor(error: ERROR) { | ||
| this.error = error | ||
| this.errorType = 'EvmError' | ||
| export class EvmError extends EthereumJSError<EvmErrorType> { |
There was a problem hiding this comment.
Side note, but I think we might want to migrate to the EVM (rather than Evm) capitalization as we've done for some others (Json -> JSON, etc.)
I like the approach, as it has the obvious benefits of:
Some of the things that I'm questioning however:
Do you have some examples on how Lodestar handles custom error codes in similar contexts (multiple error codes within a single package, with different types for each of them?). I'Ve looked but can only find examples like this: https://github.com/ChainSafe/lodestar/blob/dad9037e7739d5bcbccfe627e715ef40e9ba935b/packages/api/src/utils/server/error.ts which are simpler than what we do. Nice work! |
|
For performance, this should not have any noticeable impact, the errors are just "nicer" now since it gives error codes and error context (as example here in the REVERT error). I will clean up this PR to reflect how this would work, and if all is fine we can roll this all repo-wide. |
Great! Looking at Lodestar's handling I think it looks super clean, for e.g. this type is really readable and it would be easy to add new error types etc. : https://github.com/ChainSafe/lodestar/blob/unstable/packages/beacon-node/src/chain/errors/blockError.ts#L74 Let me know once this is ready for an actual review, excited about rolling this out progressively throughout the monorepo! |
ee33f56 to
293f4cc
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|

Part of #3712
Excerpt from
evm/errors.ts:This is a Proof-of-Concept (I still need to do some cycles to clean this up though) which shows that we now have two EVM error types: a
UNSUPPORTED_FEATUREor aRUNTIME_ERROR. In case of aRUNTIME_ERROR, typescript now demands areasonfield in the error (this could for instance be "stack overflow", "stack underflow", etc.What is nice: for the
REVERTreason it demands arevertBytesfield, which is actually the reverted bytes in case the EVM runs into a revert (from these bytes error messages can be derived, such as solidity revert strings!).Errors now look like this:
(I had added the
error.codefield but I think we should keep it aterror.type.code).The error handler (taken / inspired from lodestar: https://github.com/ChainSafe/lodestar/blob/unstable/packages/utils/src/errors.ts) allows to add custom error classes (see
EvmErrorclass in this case, can add extra constructor arguments for environment situation), and also has amessageargument which allows for custom message strings.Note: super WIP. I am not completely satisfied yet with the results, because now EVM has a huge code bloat due to instantiating these error messages. I have to find a way to go back to the "old versions" (
trap(ERROR.OUT_OF_GAS)) but still remaining type-safety (for the REVERT error string for instance).Note, tests will likely fail because of the updated error format.